Skip to content

Sandbox template rendering; cover the non-raising failure (#104) - #447

Merged
jeremymanning merged 1 commit into
mainfrom
fix/template-sandbox-ssti
Aug 2, 2026
Merged

Sandbox template rendering; cover the non-raising failure (#104)#447
jeremymanning merged 1 commit into
mainfrom
fix/template-sandbox-ssti

Conversation

@jeremymanning

Copy link
Copy Markdown
Member

Red-teaming the supported examples with subagents found a template injection.
Mutation-testing the acceptance suite found a hole it could not have seen.

Template injection, reachable from a CLI argument

A pipeline's {{ }} expressions are authored with the pipeline. The values
substituted into them are not — they arrive from -i name=value, an inputs
file, or an upstream step's output. Those values are rendered too, which is
deliberate (-i out_dir='{{ base }}/reports' is useful). Jinja's stock
Environment rendered them with Python's object graph in reach:

orchestrator run examples/supported/01_hello_filesystem.yaml \
  -i greeting='{{ "".__class__.__mro__[1].__subclasses__() | length }}'

output/greeting.txt == "1183, world"     exit 0

1183 is the length of __subclasses__() — the first hop of the standard
Jinja sandbox escape, from a plain command-line argument.

Every environment on the execution path is now built by
core/template_sandbox.py and is a SandboxedEnvironment. Three call sites
built a bare Template(...) and bypassed every environment — including the
declared-outputs renderer in orchestrator.py — and now use the same
factory.

TemplateManager.render then swallowed the SecurityError and returned the
payload unrendered, leaving the refusal visible only in a log line. A sandbox
violation now propagates; every other render failure keeps its existing
fallback.

Ordinary expressions, filters and {{ step.result.field }} are unaffected —
the blocking suite is unchanged at 562 passed, and the example catalog still
compiles everything it compiled before.

A step can fail without raising, and nothing tested it

Deleting the reported_failure branch of StepResult.from_task — the branch
catching a tool that returns {"success": false} while its task ends
COMPLETED — left the entire acceptance suite green. 06 fails by raising,
so it never reaches that code.

05_reported_failure.yaml is the sibling that does. The run exits 1 and
reports success=false while no task is FAILED:

before:       status=completed  success=True
read_missing: status=completed  success=False   File not found: ...
after:        status=completed  success=True

Case could not express that either: it assumed status completed implied
success — the same conflation the runtime had. It now carries
reported_failure separately and asserts status and success together.

Evidence

Every new assertion was mutation-tested against un-fixed code:

mutation result
un-sandbox the environment factory 7 failed
remove the SecurityError re-raise (sandbox still on) 6 failed
drop the reported_failure branch 2 failed (previously survived)
  • Blocking suite 548 → 562 passed, 0 failed
  • Examples validating 8/116 → 9/117
  • ruff check src/orchestrator (CI rule set) clean; compileall clean

Two red-team claims I could not confirm, and did not act on

  • Unresolved {{ }} reaching a file via the render fallback: real in code,
    but refuted end to end. {{ 1/0 }} exits 1 with no artifact — the
    unresolved-template guard from Stop unresolved templates before they reach a tool (#153) #439 catches it first.
  • A "nondeterministic condition operator" in 04 — the source flipping
    between > and < mid-run. That was my own doing: a mutation-testing agent
    was concurrently applying and reverting exactly that edit to that file.
    Contamination from parallel red-teaming, not a defect.

Path traversal via -i out_dir=../escape is real but I judge it by design:
this is a local CLI and the operator supplies the path. Flagging, not fixing.

Red-teaming the supported examples turned up a template injection, and
mutation-testing the acceptance suite turned up a hole it could not see.

**Parameter values were executed with Python's object graph in reach.**
A pipeline's `{{ }}` expressions are authored with the pipeline; the values
substituted into them are not, arriving from `-i name=value`, an inputs file,
or an upstream step. Those values are rendered too -- deliberately, since
`-i out_dir='{{ base }}/reports'` is useful -- but Jinja's stock `Environment`
renders them unrestricted:

    orchestrator run examples/supported/01_hello_filesystem.yaml \
      -i greeting='{{ "".__class__.__mro__[1].__subclasses__() | length }}'
    -> output/greeting.txt == "1183, world", exit 0

That is the first hop of the standard Jinja sandbox escape, reachable from a
plain CLI argument. Every environment on the execution path is now built by
`core/template_sandbox.py` and is a `SandboxedEnvironment`. Three call sites
were building a bare `Template(...)`, bypassing every environment including
the declared-outputs renderer in `orchestrator.py`; they go through the same
factory now. Ordinary expressions, filters and `{{ step.result.field }}` are
unaffected -- the blocking suite is unchanged at 562 passed.

`TemplateManager.render` swallowed the resulting `SecurityError` and returned
the payload unrendered, leaving the refusal visible only in a log line. A
sandbox violation now propagates; every other render failure keeps its
existing fallback.

**A step can fail without raising, and nothing tested it.**
Deleting the `reported_failure` branch of `StepResult.from_task` -- the branch
that catches a tool returning `{"success": false}` while its task ends
COMPLETED -- left the entire acceptance suite green. 06 fails by raising, so
it never reaches that code. `05_reported_failure.yaml` is the sibling that
does: the run exits 1 and reports success=false while no task is FAILED.

`Case` could not express it either. It assumed status "completed" implied
success, the same conflation the runtime had; it now carries
`reported_failure` separately and asserts both fields together.

Each new assertion was mutation-tested against un-fixed code: un-sandboxing
the factory fails 7, removing the SecurityError re-raise fails 6, and dropping
the `reported_failure` branch now fails 2 where it previously survived.

Examples validating: 8/116 -> 9/117. Blocking suite: 548 -> 562 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeremymanning

Copy link
Copy Markdown
Member Author

CI verification

9/9 green.

Legacy tally against the current post-merge main run (30765313676, which
already includes #446):

main #447 delta
failed 474 473 -1
passed 1846 1847 +1
errors 189 189 0
skipped 200 200 0
deselected 647 661 +14

The +14 deselected is exactly the 14 tests this PR adds to the blocking layer
(548 -> 562), which the legacy job runs the complement of. The -1/+1 is inside
the +/-3 noise floor, so I am not claiming it.

The one number that moved unexplained

Warnings went 11 -> 12. Rather than wave that past as noise, I compared the
warning classes in both logs:

main   ->  2 RuntimeWarning, 1 PytestUnraisableExceptionWarning, 1 PytestReturnNotNoneWarning
#447   ->  2 RuntimeWarning, 1 PytestUnraisableExceptionWarning, 1 PytestReturnNotNoneWarning

Identical. The extra warning is another instance of a class already present,
not a new kind of problem introduced here. Sandboxing the environments does
not raise anything new in the legacy layer.

Worth noting that PytestUnraisableExceptionWarning is still there. That is
the same class #446 fixed in TerminalTool for the blocking layer, which
supports what I said on that PR: there is at least one more abandoned
subprocess or unclosed transport in the legacy surface, and it is still
unfiled.

@jeremymanning
jeremymanning merged commit de93ae6 into main Aug 2, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant